TNTP-3334: IPROTO_IS_SYNC support for begin/commit #447

bigbes · 2025-06-27T08:51:22Z

Implement BeginRequest.IsSync(bool) and CommitRequest.IsSync(bool).

Refactor Body methods of all Requests, do not use fillXXX functions.

Part of TNTP-3334
Closes #366

oleg-jukovec · 2025-07-01T10:15:19Z

Closes #TNTP-3334

->

Part of #TNTP-3334

oleg-jukovec · 2025-07-01T19:20:14Z

example_test.go

+	conn := exampleConnect(dialer, opts)
+	defer conn.Close()
+
+	// Tarantool supports IS_SYNC flag for BeginRequest since version 3.1.0


A comment should end with a dot.

Suggested change

// Tarantool supports IS_SYNC flag for BeginRequest since version 3.1.0

// Tarantool supports IS_SYNC flag for BeginRequest since version 3.1.0.

oleg-jukovec · 2025-07-01T19:20:27Z

example_test.go

+	conn := exampleConnect(dialer, opts)
+	defer conn.Close()
+
+	// Tarantool supports IS_SYNC flag for CommitRequest since version 3.1.0q


Suggested change

// Tarantool supports IS_SYNC flag for CommitRequest since version 3.1.0q

// Tarantool supports IS_SYNC flag for CommitRequest since version 3.1.0.

oleg-jukovec · 2025-07-01T19:20:33Z

example_test.go

+		return
+	}
+
+	// Begin transaction


Suggested change

// Begin transaction

// Begin transaction.

oleg-jukovec · 2025-07-01T19:20:42Z

example_test.go

+		return
+	}
+
+	// Insert in stream


Suggested change

// Insert in stream

// Insert in stream.

oleg-jukovec · 2025-07-01T19:20:48Z

example_test.go

+		return
+	}
+
+	// Commit transaction in sync mode


Suggested change

// Commit transaction in sync mode

// Commit transaction in sync mode.

oleg-jukovec · 2025-07-01T19:21:45Z

stream.go

+	baseRequest
+	txnIsolation TxnIsolationLevel
+	timeout      time.Duration
+	isSync       *bool


Let's avoud a pointer here.

Suggested change

isSync *bool

isSync bool

isSyncExist bool

oleg-jukovec · 2025-07-01T19:22:08Z

stream.go

@@ -137,6 +152,8 @@ func (req *BeginRequest) Context(ctx context.Context) *BeginRequest {
 // Commit request can not be processed out of stream.
 type CommitRequest struct {
 	baseRequest
+
+	isSync *bool


dmyger · 2025-07-02T13:04:08Z

prepared.go

@@ -171,8 +167,24 @@ func (req *ExecutePreparedRequest) Args(args interface{}) *ExecutePreparedReques
 }

 // Body fills an msgpack.Encoder with the execute request body.
-func (req *ExecutePreparedRequest) Body(res SchemaResolver, enc *msgpack.Encoder) error {
-	return fillExecutePrepared(enc, *req.stmt, req.args)
+func (req *ExecutePreparedRequest) Body(_ SchemaResolver, enc *msgpack.Encoder) error {


For here and all other functions.
Just a cosmetic: it would a bit better to add empty lines between blocks.
At least if we add linters from tt they will force add such lines.

dmyger · 2025-07-02T13:04:18Z

prepared.go

+	if err != nil {
+		return err
+	}
+	err = enc.EncodeUint(uint64(iproto.IPROTO_STMT_ID))


Suggested change

err = enc.EncodeUint(uint64(iproto.IPROTO_STMT_ID))

err = enc.EncodeUint(uint64(iproto.IPROTO_STMT_ID))

dmyger · 2025-07-02T13:04:52Z

prepared.go

+	if err != nil {
+		return err
+	}
+	err = enc.EncodeUint(uint64(req.stmt.StatementID))


Suggested change

err = enc.EncodeUint(uint64(req.stmt.StatementID))

err = enc.EncodeUint(uint64(req.stmt.StatementID))

dmyger · 2025-07-02T13:05:03Z

prepared.go

+	if err != nil {
+		return err
+	}
+	err = enc.EncodeUint(uint64(iproto.IPROTO_SQL_BIND))


Suggested change

err = enc.EncodeUint(uint64(iproto.IPROTO_SQL_BIND))

err = enc.EncodeUint(uint64(iproto.IPROTO_SQL_BIND))

dmyger · 2025-07-02T13:05:15Z

prepared.go

+	if err != nil {
+		return err
+	}
+	return encodeSQLBind(enc, req.args)


Suggested change

return encodeSQLBind(enc, req.args)

return encodeSQLBind(enc, req.args)

oleg-jukovec

The dot is missed.

NB: all deleted tests were checking that functions passed from
constructor object to fillXXX function were passed in right order, so
removing them is not a problem

->

NB: all deleted tests were checking that functions passed from
constructor object to fillXXX function were passed in right order, so
removing them is not a problem.

example_test.go

prepared.go

NB: all deleted tests were checking that functions passed from constructor object to fillXXX function were passed in right order, so removing them is not a problem.

Part of #TNTP-3334 Closes #366

oleg-jukovec

LGTM, please don't forget to add a removed tests analogues in a next PR.

bigbes force-pushed the bigbes/TNTP-3334-iproto-is-sync-support branch 5 times, most recently from 5f1d4dc to 4357337 Compare June 30, 2025 14:57

bigbes requested review from dmyger and oleg-jukovec June 30, 2025 14:58

bigbes marked this pull request as ready for review June 30, 2025 15:00

bigbes force-pushed the bigbes/TNTP-3334-iproto-is-sync-support branch from 4357337 to c74d03f Compare June 30, 2025 15:03

bigbes force-pushed the bigbes/TNTP-3334-iproto-is-sync-support branch 3 times, most recently from 8892063 to f8f4d2d Compare July 1, 2025 12:55

oleg-jukovec requested changes Jul 1, 2025

View reviewed changes

bigbes force-pushed the bigbes/TNTP-3334-iproto-is-sync-support branch 2 times, most recently from dea5ad8 to 168a18a Compare July 2, 2025 10:19

bigbes requested a review from oleg-jukovec July 2, 2025 11:04

dmyger approved these changes Jul 2, 2025

View reviewed changes

oleg-jukovec requested changes Jul 2, 2025

View reviewed changes

example_test.go Outdated Show resolved Hide resolved

prepared.go Outdated Show resolved Hide resolved

bigbes added 2 commits July 3, 2025 10:20

refactor: get rid of fillXXX functions, in favor of Body method

4de68c4

NB: all deleted tests were checking that functions passed from constructor object to fillXXX function were passed in right order, so removing them is not a problem.

stream: added is_sync flag for begin/commit

57d71e8

Part of #TNTP-3334 Closes #366

bigbes force-pushed the bigbes/TNTP-3334-iproto-is-sync-support branch from 168a18a to 57d71e8 Compare July 3, 2025 07:22

oleg-jukovec approved these changes Jul 3, 2025

View reviewed changes

oleg-jukovec merged commit fba773c into master Jul 3, 2025
24 of 26 checks passed

oleg-jukovec deleted the bigbes/TNTP-3334-iproto-is-sync-support branch July 3, 2025 10:11

	// Tarantool supports IS_SYNC flag for BeginRequest since version 3.1.0
	// Tarantool supports IS_SYNC flag for BeginRequest since version 3.1.0.

	// Tarantool supports IS_SYNC flag for CommitRequest since version 3.1.0q
	// Tarantool supports IS_SYNC flag for CommitRequest since version 3.1.0.

	// Commit transaction in sync mode
	// Commit transaction in sync mode.

	err = enc.EncodeUint(uint64(iproto.IPROTO_STMT_ID))

	err = enc.EncodeUint(uint64(iproto.IPROTO_STMT_ID))

	err = enc.EncodeUint(uint64(req.stmt.StatementID))

	err = enc.EncodeUint(uint64(req.stmt.StatementID))

	return encodeSQLBind(enc, req.args)

	return encodeSQLBind(enc, req.args)

TNTP-3334: IPROTO_IS_SYNC support for begin/commit #447

TNTP-3334: IPROTO_IS_SYNC support for begin/commit #447

Conversation

bigbes commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oleg-jukovec commented Jul 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleg-jukovec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

oleg-jukovec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bigbes commented Jun 27, 2025 •

edited

Loading